Skip to content

msggen: add multiwithdraw method - #7271

Closed
daywalker90 wants to merge 1 commit into
ElementsProject:masterfrom
daywalker90:multiwithdraw
Closed

msggen: add multiwithdraw method#7271
daywalker90 wants to merge 1 commit into
ElementsProject:masterfrom
daywalker90:multiwithdraw

Conversation

@daywalker90

Copy link
Copy Markdown
Collaborator

nothing special, adds:

  • multiwithdraw

@daywalker90
daywalker90 requested a review from cdecker as a code owner April 24, 2024 13:20
@cdecker

cdecker commented Apr 24, 2024

Copy link
Copy Markdown
Member

These PRs would be much simpler to review and merge if they weren't this disjoint by the way. The way you are doing it at the moment, the .msggen.json gets reset inbetween runs, which can cause the CI to fail on master and we have to go back and refill them in.

Can you maybe create 5-method chunks? That'd also cut down on the reviewing overhead (I get notified for every single update to any of these PRs, which de-sensitivizes reviewers to notifications, and stuff gets lost in the torrent of notifications).

@cdecker

cdecker commented Apr 24, 2024

Copy link
Copy Markdown
Member

Keep the changes to one method per commit, even when clustering, otherwise we lose the nice collation of manual changes and derived changes.

@cdecker

cdecker commented Apr 24, 2024

Copy link
Copy Markdown
Member

Also, CI resources aren't free, so let's try to preserve them as much as possible, even if it is GH footing the bill, we should be respectful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants